fix(storage): reject a data directory from another network by MegaRedHand · Pull Request #556 · lambdaclass/ethlambda

MegaRedHand · 2026-07-31T21:31:37Z

Stacked on #554 (fix/resume-db-without-checkpoint-url). Base is that branch, so this diff is just the genesis check. Merge #554 first.

🗒️ Description / Motivation

Store::from_db_state decided whether a data directory was ours by comparing genesis_time alone, read from the persisted ChainConfig, and on mismatch logged a warning and returned None — "treat as empty":

if persisted_config.genesis_time != expected_genesis_time {
    warn!(..., "Persisted DB has a different genesis_time; treating as empty");
    return Ok(None);
}

Two problems with that.

1. "Treat as empty" is not empty. The caller then wrote a fresh anchor over the foreign chain's rows without clearing them. get_signed_blocks_by_slot_range resolves each slot through BlockRoots with no anchor check:

for slot in start_slot..=end_slot {
    let Some(root_bytes) = view.get(Table::BlockRoots, &encode_block_root_key(slot))? else { continue };
    // ... no check that this root belongs to the current chain
}

so for slots the new chain had not reached yet, BlocksByRange served the other network's blocks to peers, who reject them and score-penalize us. (Fork choice is unaffected: compute_lmd_ghost_head is seeded from latest_justified, so foreign roots are unreachable and never accumulate weight.)

2. ChainConfig is { genesis_time: u64 }. A network regenerated with the same genesis time but a different validator set was not detected at all — the DB was resumed as if it were ours. #554 makes the DB readable without --checkpoint-sync-url, which puts that case on the default restart path.

What Changed

crates/common/types/src/genesis.rs — new verify_state_genesis(state, genesis_time, expected_validators) plus a GenesisConfig::verify_state convenience wrapper, and a GenesisMismatch error enum. Compares genesis time and the full validator registry: count, sequential indices, and both pubkeys per validator. The validator set is fixed at genesis (nothing in the state transition mutates it), so any state of our chain must carry exactly that registry, whatever slot it sits at.

crates/storage/src/store.rs — from_db_state takes &GenesisConfig and verifies the finalized state (never pruned, and the state the anchor is rebuilt from) rather than the persisted config. A mismatch is now Error::GenesisMismatch. A missing finalized state is Error::UnexpectedMissingState instead of silently "empty".

bin/ethlambda/src/checkpoint_sync.rs — verify_checkpoint_state had its own copy of the same four checks; it now delegates to the shared function, and GenesisTimeMismatch / ValidatorCountMismatch / NonSequentialValidatorIndex / ValidatorPubkeyMismatch collapse into one Genesis(#[from] GenesisMismatch). Its checkpoint-only sanity checks stay put — slot != 0 must not apply to the DB path, since a data directory legitimately sits at genesis while a downloaded anchor never does.

crates/storage/src/lib.rs — export Error. It was a private type appearing in public signatures, so callers could not name it to match on it.

bin/ethlambda/src/main.rs — fetch_initial_state propagates the error (if let Some(store) = Store::from_db_state(..)?) instead of swallowing it with if let Ok(Some(..)).

Correctness / Behavior Guarantees

DB in data directory	before	after
Ours	resumed	resumed
Different `GENESIS_TIME`	warn → new anchor written over foreign rows	startup aborts, DB untouched
Same `GENESIS_TIME`, different validator set	undetected, resumed as ours	startup aborts
Genuinely empty	initialize	initialize (unchanged)

Aborting is deliberate rather than falling back: there is no safe way to reuse the directory, so the operator has to fix --data-dir or remove it. The error names what differs, e.g. persisted state does not match the configured genesis: validator 1 pubkey mismatch (attestation or proposal key).

Operational note for reviewers: any flow that reuses a data directory across a genesis regeneration now fails to boot loudly instead of silently restarting from a fresh anchor. lean-quickstart's --generateGenesis implies --cleanData, so local devnets are unaffected; ansible/Hive flows that regenerate genesis in place would need to clear the directory.

Tests Added / Run

crates/common/types — 5 tests on verify_state: accepts a state from the same genesis; rejects different genesis time, different validator count, swapped validator keys at the same count and genesis time, and non-sequential indices.

crates/storage — from_db_state_errors_on_genesis_time_mismatch and from_db_state_errors_on_validator_set_mismatch.

bin/ethlambda — fails_when_db_genesis_time_differs (also asserts the original DB still loads under its own genesis, i.e. it was not overwritten) and fails_when_db_validator_set_differs.

One test changed contract rather than being fixed: from_db_state_returns_none_on_genesis_time_mismatch encoded the "treat as empty" behavior this PR removes. It is replaced by the two ..._errors_on_... tests above. Flagging explicitly since that is a deliberate contract change, not a broken test.

make fmt && make lint && make test    # 30 test binaries, 0 failures

Related Issues / PRs

Stacked on fix(startup): resume from existing DB without a checkpoint URL #554

✅ Verification Checklist

Ran make fmt — clean
Ran make lint (clippy with -D warnings) — clean
Ran make test (cargo test --workspace --profile release-fast) — all passing

## 🗒️ Description / Motivation Restarting a node without `--checkpoint-sync-url` destroyed its chain. `fetch_initial_state` gated the on-disk state lookup on that flag: ```rust if checkpoint_urls.is_empty() { info!("No checkpoint sync URL provided, initializing from genesis state"); let genesis_state = State::from_genesis(genesis.genesis_time, validators); return Ok(Store::from_anchor_state(backend, genesis_state)); }; // ... only past this point was Store::from_db_state tried ``` So a redeploy against a populated RocksDB wrote a slot-0 genesis anchor over a perfectly current chain, and operators had to pass a checkpoint URL purely as a *fallback trigger* even when the DB was fresh and the URL was never fetched. This makes on-disk state authoritative: `--checkpoint-sync-url` becomes a fallback for when there is nothing resumable, not a precondition for reading what is there. ## What Changed **`bin/ethlambda/src/main.rs`** — `fetch_initial_state` tries `Store::from_db_state` *before* the empty-URL early return: ``` gap = current_slot − store.head_slot() gap ≤ MAX_RESUMABLE_DB_STATE_AGE → resume from DB info! gap > MAX, no checkpoint URLs → resume from DB warn! (new) gap > MAX, checkpoint URLs set → fall through to checkpoint sync no resumable DB, checkpoint URLs → checkpoint sync no resumable DB, no URLs → genesis ``` Also removes the `info!(url_count, "Starting checkpoint sync")` that was emitted *before* the DB was consulted. It fired on every successful resume, so grepping a boot log for `"Starting checkpoint sync"` false-positived on nodes that never synced. Each outcome now logs exactly one line, at the point the decision is made: | Boot log line | Outcome | | --- | --- | | `Resuming from existing DB head_slot=… current_slot=… gap=…` | Resumed from disk, nothing downloaded | | `DB is stale; resuming anyway head_slot=… current_slot=… gap=…` | Resumed past the window, no URL to prefer | | `DB is stale; checkpoint sync head_slot=… current_slot=… gap=…` | Past the window, a URL took over | | `Starting checkpoint sync checkpoint_urls=[…]` | Downloading a checkpoint | | `No checkpoint sync URL provided, initializing from genesis state` | Started from genesis | **`bin/ethlambda/src/cli.rs`** — `--checkpoint-sync-url` help text no longer claims it "skips genesis initialization"; it is documented as a fallback. **`docs/checkpoint_sync.md`** — new *Restarts and Existing State* section: precedence table, the resume window and why it is measured against the head rather than the finalized checkpoint, the P2P-catch-up caveat, and why an all-URLs-fail abort is intentional. ## Correctness / Behavior Guarantees | DB state (matching `GENESIS_TIME`) | `--checkpoint-sync-url` | before | after | | --- | --- | --- | --- | | absent | omitted | genesis | genesis | | absent | set | checkpoint sync | checkpoint sync | | fresh (head-lag ≤ 450) | omitted | **genesis, resets to slot 0** | **resume** | | fresh | set | resume | resume | | stale (head-lag > 450) | omitted | **genesis, resets to slot 0** | **resume + warning** | | stale | set | checkpoint sync | checkpoint sync | - `MAX_RESUMABLE_DB_STATE_AGE` keeps its value and its meaning in the URL-present case; it now only decides *whether a checkpoint is preferable to what we already have*, never whether the DB is readable. - Staleness is still measured against the head (`current_slot - head_slot`), so a node whose head is current resumes during a finality stall. - **Stale DB + no URL resumes rather than refusing to boot.** No checkpoint URL was configured, so there is no anchor to switch to and the node runs against the data directory it was given. The warning exists because range sync may not close a gap this large: peers prune block signatures past `SIGNATURE_PRUNING_RANGE` (~1 day), so beyond that horizon they cannot serve the missing history and the node needs a checkpoint URL. Refusing to start instead would break unattended restarts after a routine 31-minute outage. - **Stale DB + URLs set + every URL failing still aborts.** Deliberate, and documented as such: configuring the flag asks for a specific anchor, so an unreachable source is a misconfiguration to surface at boot rather than paper over by starting a node that is hours behind. Omitting the flag is how you ask for "resume whatever is on disk"; that path never aborts. - **No new flag.** Omitting `--checkpoint-sync-url` no longer means "start from genesis" when a DB exists; to deliberately start over, remove the data directory. That is already the documented idiom for a clean checkpoint sync, and an `--ignore-existing-db` flag would only reintroduce the write-genesis-over-live-data footgun behind a flag. - **Unchanged / out of scope:** a `GENESIS_TIME` mismatch still degrades silently (`from_db_state` logs `"Persisted DB has a different genesis_time; treating as empty"`), so with no URL the node writes genesis over a foreign-network DB. Pre-existing behavior, addressed separately in #556; `initializes_from_genesis_when_db_genesis_time_differs` pins it here as a known hazard rather than a desired invariant. ## Tests Added / Run Six unit tests in `bin/ethlambda/src/main.rs` driving `fetch_initial_state` against `InMemoryBackend`: | Test | Gap | Asserts | | --- | --- | --- | | `initializes_from_genesis_when_db_is_empty` | — | head slot 0 | | `resumes_from_fresh_db_without_checkpoint_url` | `MAX / 2` | head slot is the seeded slot, not 0 | | `resumes_from_stale_db_without_checkpoint_url` | `MAX + 100` | resumes despite `gap > MAX_RESUMABLE_DB_STATE_AGE` | | `resumes_from_fresh_db_with_checkpoint_url` | `= MAX` | resume wins over a URL; nothing is dialed | | `falls_through_to_checkpoint_sync_when_db_is_stale` | `MAX + 1` | past the window the URL takes over, and an unreachable one aborts | | `initializes_from_genesis_when_db_genesis_time_differs` | — | head slot 0 (DB treated as empty) | The seeded anchor sits above slot 0 because a genesis re-init also yields head slot 0; that is what makes "resumed" distinguishable from "started over". Staleness is induced purely by choosing `genesis_time` (`current_slot` derives from the wall clock against it), so no clock injection. The two no-URL resume tests cannot pin the threshold on their own: both no-URL branches return the same store, so inverting the comparison leaves them green. The pair that can are the two with a URL set, where the outcomes differ. Verified by mutation: | Mutation | Result | | --- | --- | | `gap <= MAX` → `gap > MAX` | both URL tests fail, the four others pass | | `gap <= MAX` → `gap < MAX` | the `= MAX` boundary test fails | Those two use `#[tokio::test(start_paused = true)]` so the checkpoint retry backoff (5 attempts × 5s) costs no wall clock; the connection refusal against `http://127.0.0.1:1` is immediate. That needs tokio's `test-util` feature as a **dev**-dependency, so it never reaches the shipped binary. Commands run: ``` cargo test -p ethlambda --profile release-fast --bin ethlambda # 33 passed make fmt && make lint && make test # all clean (550 passed, 7 pre-existing ignored) ``` Local multi-client devnet verification is in progress; I'll post the boot logs showing a keep-DB restart with no `--checkpoint-sync-url` as a comment. ## Related Issues / PRs - Related to #505 (head-lag resume gate, which this builds on) - #556 rejects a data directory from another network, covering the `GENESIS_TIME`-mismatch hazard this PR only pins - #560 carries the unrelated `CLAUDE.md` RPC-port note that was originally in this branch - #559 tracks a pre-existing bug this PR makes easier to hit: the duty sync gate reports Synced while a node backfills from a stale resume, so it attests and proposes on an old head. Not addressed here - Logging a `Store::from_db_state` read error instead of discarding it (the `Err` arm of `if let Ok(Some(_))`, unreachable today) is left to a follow-up PR ## ✅ Verification Checklist - [x] Ran `make fmt` — clean - [x] Ran `make lint` (clippy with `-D warnings`) — clean - [x] Ran `make test` (`cargo test --workspace --profile release-fast`) — all passing

`Store::from_db_state` compared only `genesis_time`, taken from the persisted `ChainConfig`, and on mismatch logged a warning and returned `None` — "treat as empty". The caller then wrote a fresh anchor over the foreign chain's rows without clearing them. `get_signed_blocks_by_slot_range` resolves slots through `BlockRoots` with no anchor check, so for slots the new chain had not reached yet `BlocksByRange` served the other network's blocks to peers, who reject them and penalize our score. Worse, `ChainConfig` carries only `genesis_time`, so a network regenerated with the same genesis time but a different validator set was not detected at all: the DB was resumed as if it were ours. Making the DB readable without `--checkpoint-sync-url` (previous commit) puts that case on the default restart path. Compare the whole genesis instead — genesis time plus the full validator registry (count, sequential indices, both pubkeys) — against the finalized state rather than the persisted config, and make a mismatch fatal. The validator set is fixed at genesis, so any state of our chain must carry exactly that registry. Refusing to boot is deliberate: there is no safe way to reuse the directory, and the operator needs to fix --data-dir or remove it. The comparison lives in `ethlambda-types` as `verify_state_genesis`, shared with `checkpoint_sync::verify_checkpoint_state`, which had its own copy of the same four checks; its `GenesisTimeMismatch`/`ValidatorCountMismatch`/ `NonSequentialValidatorIndex`/`ValidatorPubkeyMismatch` variants collapse into one `Genesis(#[from] GenesisMismatch)`. The checkpoint-only sanity checks (slot != 0, finalized <= slot, header pairing) stay there, since a DB legitimately sits at genesis while a downloaded anchor never does. `from_db_state_returns_none_on_genesis_time_mismatch` asserted the old "treat as empty" contract and is replaced by two tests asserting the new fatal one, one of them covering the same-genesis-time case the old check could not see.

github-actions · 2026-08-03T22:16:54Z

🤖 Kimi Code Review

This PR introduces a critical security fix for network identity verification while refactoring genesis validation into a shared, testable module. The changes prevent accidental cross-network data corruption and ensure validator registry integrity.

Security & Correctness

Critical Fix: Foreign Network Detection

File: crates/storage/src/store.rs (lines 647-666)
Issue Resolved: Previously, from_db_state only checked genesis_time and silently treated mismatches as empty databases (with just a warning). This allowed a node to potentially re-initialize on top of a foreign chain's data, leaving orphaned blocks that could be served to peers via BlocksByRange.
Fix: Now validates the full validator registry (count, sequential indices, and both XMSS pubkeys) against the configured genesis. Returns Error::GenesisMismatch as a hard error, aborting startup.

Checkpoint Sync Validation

File: bin/ethlambda/src/checkpoint_sync.rs (lines 202-205)
Improvement: Replaces inline validation with shared verify_state_genesis function, ensuring checkpoint sync and disk-resume paths use identical network-identity checks. The Genesis error variant (line 49) properly encapsulates all mismatch scenarios.

Code Quality

Error Handling

File: crates/common/types/src/genesis.rs (lines 7-21)
The new GenesisMismatch enum provides structured error contexts for debugging. The #[from] implementations in CheckpointSyncError and Error enable ergonomic propagation.

Performance

File: crates/storage/src/store.rs (lines 657-666)
Loading the full finalized state at startup to verify validators is O(n) in validator count. For networks with large validator sets (e.g., 1M+), this adds startup overhead but is acceptable given the security requirements. Consider documenting this cost.

Testing

File: crates/storage/src/store.rs (lines 2993-3060)
Excellent test coverage for the three cases: empty DB, matching genesis, and both types of mismatch (time vs. validators).
File: bin/ethlambda/src/main.rs (lines 981-1022)
Integration tests verify that foreign databases are neither loaded nor overwritten.

Minor Observations

Error Message Clarity
- File: crates/common/types/src/genesis.rs (line 13)
- The format string "(expected {position}, got {got})" is correct since position equals the expected index, but consider renaming the field to expected in NonSequentialIndex for clarity, or adjust the template to avoid confusion.
State Retrieval Safety
- File: crates/storage/src/store.rs (line 661)
- Using latest_finalized to load the verification state assumes the finalized state is never pruned. Ensure this invariant holds in your pruning strategy, or document that the latest finalized state must be retained.
Iterator Safety
- File: crates/common/types/src/genesis.rs (line 70)
- The zip usage is safe due to the preceding length check, but consider adding a debug assertion or using zip_eq (from itertools) in future to panic on length mismatch in debug builds.

Consensus Considerations

The validator registry verification correctly enforces Ethereum's network identity definition:

Genesis time and validator set are the canonical network identifiers
Sequential index validation (0..n) ensures state integrity
Dual pubkey comparison (attestation + proposal) aligns with 3SF-mini's XMSS requirements

Acknowledgment: The refactoring effectively deduplicates validation logic between sync and storage layers, reducing maintenance burden and preventing divergence in security-critical checks.

Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt

github-actions · 2026-08-03T22:18:08Z

🤖 Codex Code Review

High: the new startup guard only proves that the finalized state matches the configured genesis; it does not prove that the persisted canonical indices are clean. Store::from_db_state accepts the DB after genesis.verify_state(&state) on the finalized state (crates/storage/src/store.rs), but BlocksByRange is still served directly from Table::BlockRoots (crates/storage/src/store.rs, crates/net/p2p/src/req_resp/handlers.rs). Since init_store writes new anchor metadata/index rows without clearing old ones (crates/storage/src/store.rs), a data dir that was already contaminated by the previous “re-anchor over foreign DB” behavior can still pass this new check and continue serving stale foreign canonical blocks. If the intent is to close that security hole, startup needs either a stronger integrity check over the canonical chain/index tables or a one-time cleanup/migration path for previously mixed directories.

No other correctness issues stood out in the diff; the shared genesis-identity check itself is a solid improvement.

I couldn’t run the Rust tests here because this environment has a read-only Cargo/rustup home and no network access, so this is a static review only.

Automated review by OpenAI Codex · gpt-5.4 · custom prompt

greptile-apps · 2026-08-03T22:20:02Z

Greptile Summary

This PR prevents a node from reusing a data directory belonging to another network.

Adds shared genesis identity verification covering genesis time, validator count, sequential indices, and validator public keys.
Applies that verification to persisted finalized state and checkpoint-sync state.
Propagates storage identity and missing-state errors to abort startup instead of overwriting foreign data.
Exports the storage error type and documents the new startup behavior.

Confidence Score: 5/5

The PR appears safe to merge, with no concrete changed-code defect identified.

The shared identity validation matches the state model’s immutable validator registry, persisted-state failures now propagate before any replacement anchor is written, and checkpoint-specific sanity checks remain intact.

Important Files Changed

Filename	Overview
crates/common/types/src/genesis.rs	Adds a shared, well-tested genesis identity check over genesis time and the immutable validator registry.
crates/storage/src/store.rs	Validates the persisted finalized state before resuming and returns explicit errors rather than treating foreign data as empty.
bin/ethlambda/src/checkpoint_sync.rs	Reuses the shared genesis verifier while retaining checkpoint-specific structural checks.
bin/ethlambda/src/main.rs	Propagates persisted-store errors so foreign data aborts startup without being overwritten.
crates/storage/src/error.rs	Adds explicit errors for missing finalized state and genesis mismatch.
crates/storage/src/lib.rs	Publicly exports the storage error type used by public fallible APIs.
docs/checkpoint_sync.md	Documents foreign-state rejection and the shared checkpoint/persisted-state identity checks.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Open data directory] --> B{Chain metadata present?}
  B -- No --> C[Initialize from genesis or checkpoint]
  B -- Yes --> D[Load finalized state]
  D --> E{Matches configured genesis?}
  E -- Yes --> F[Resume persisted store]
  E -- No --> G[Abort startup without modifying DB]
  D --> H{State missing or unreadable?}
  H -- Yes --> G

_{Reviews (1): Last reviewed commit: "fix(storage): reject a data directory fr..." | Re-trigger Greptile}

MegaRedHand force-pushed the fix/verify-db-genesis-matches branch from b70d070 to 1da123b Compare August 3, 2026 19:10

MegaRedHand mentioned this pull request Aug 3, 2026

fix(startup): resume from existing DB without a checkpoint URL #554

Merged

3 tasks

Base automatically changed from fix/resume-db-without-checkpoint-url to main August 3, 2026 21:59

MegaRedHand force-pushed the fix/verify-db-genesis-matches branch from 1da123b to 00908e9 Compare August 3, 2026 22:14

MegaRedHand marked this pull request as ready for review August 3, 2026 22:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(storage): reject a data directory from another network - #556

fix(storage): reject a data directory from another network#556
MegaRedHand wants to merge 1 commit into
mainfrom
fix/verify-db-genesis-matches

MegaRedHand commented Jul 31, 2026

Uh oh!

github-actions Bot commented Aug 3, 2026

Uh oh!

github-actions Bot commented Aug 3, 2026

Uh oh!

greptile-apps Bot commented Aug 3, 2026

Important Files Changed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

MegaRedHand commented Jul 31, 2026

🗒️ Description / Motivation

What Changed

Correctness / Behavior Guarantees

Tests Added / Run

Related Issues / PRs

✅ Verification Checklist

Uh oh!

github-actions Bot commented Aug 3, 2026

🤖 Kimi Code Review

Security & Correctness

Code Quality

Minor Observations

Consensus Considerations

Uh oh!

github-actions Bot commented Aug 3, 2026

🤖 Codex Code Review

Uh oh!

greptile-apps Bot commented Aug 3, 2026

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Flowchart

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant